FiberRef #1822
FiberRef #1822
Conversation
| * Divorces the current reference from parent fiber and | ||
| * sets a new reference for the duration of `fa` evaluation. | ||
| */ | ||
| def locally(fa: F[A]): F[A] |
There was a problem hiding this comment.
shouldn't that be def locally[B](fb: F[B]): F[B]? with a new method level type-parameter?
There was a problem hiding this comment.
We've had a similar case in Resource once... 0bd5660 (#1368)
| local <- FiberLocal[Ref[IO, A]](ref) | ||
| } yield new FiberRef[IO, A] { | ||
| override def locally(fa: IO[A]): IO[A] = { | ||
| val acquire = local.get.product(Ref.of[IO, A](default)).flatTap { |
There was a problem hiding this comment.
Random thought, should this use default or the current value in the "outer" scope as the initial value instead?
Also, we can save a lot of typing by doing local.get first and flatMapping over that, but I don't know about the performance.
| * Divorces the current reference from parent fiber and | ||
| * sets a new reference for the duration of `fa` evaluation. | ||
| */ | ||
| def locally(fa: F[A]): F[A] |
There was a problem hiding this comment.
I tried using locals (a copy of this PR) to get a separate scope for every test in a weaver suite, but:
- using a normal
IOLocalis insufficient, as I want to share the scope between fibers forked within a single test - using a
FiberReflike here works as long as I remember to isolate the scope. Problem is, I don't expose the locals because they're hidden behind some interface (i.e. a tagless algebra). A way to dolocallywith all FiberRefs in existence would be really cool. So pretty much theIO.clearLocalsyou mentioned in Fiber locals #1393, but with knowledge ofFiberRef(just clearing the local doesn't cut it for a Ref because it can be the same value if we're at the start of a test).
with knowledge of
FiberRef
this is probably the worst part.
|
Not sure what the status is of this PR. Is there a consensus on how to continue here? Feel free to remove it from the milestone if you think it's not going to make it. |
|
Any consensus on this? It seems like |
Follow up to #1393